[Battery Manager] - New Module #4221

HenriRabalais · 2018-12-20T21:54:59Z

Summary of Functions

This is the Battery Manager Module. It manages test batteries. It was original made by @victoriafoing. Since leaving, I have maintained and refactored the code to meet new standards, while keeping the same functionality:

Tests can be viewed and filtered via the Menu Filter.
Tests can be added via the Add Test button and form. Every new test is given a status of 'Active' === 'Y'
Tests can be activated and deactivated via the Activate and Deactivate buttons.
Tests can be "Edited" via the Edit button and form. This is not true editing, since every edit creates a new test in the database. This also causes the previous test to be deactivated.
Duplicates of "Active" Tests are not permitted. However, when a user attempts to create a duplicate of a inactive test, they are given the option to activate it.

Features

Includes new React Menu Filter Template.
Includes Real LORIS endpoints and no ajax scripts.
Uses the dataframework.

driusan · 2018-12-21T15:05:20Z

Not sure how since nothing was merged since this PR was sent yesterday, but this already has conflicts

SQL/New_patches/2018_07_23_battery_manager_permissions.sql

driusan

First pass at reviewing

jsx/Filter.js

jsx/Form.js

modules/battery_manager/README.md

modules/battery_manager/php/battery_manager.class.inc

modules/battery_manager/php/test.class.inc

modules/battery_manager/php/battery_manager.class.inc

modules/battery_manager/php/testprovisioner.class.inc

jsx/Form.js

jsx/Modal.js

modules/battery_manager/README.md

modules/battery_manager/help/battery_manager.md

modules/battery_manager/php/battery_manager.class.inc

modules/battery_manager/php/testprovisioner.class.inc

modules/battery_manager/test/TestPlan.md

HenriRabalais · 2019-01-07T22:11:15Z

@PapillonMcGill Addressed!

modules/battery_manager/test/TestPlan.md

PapillonMcGill · 2019-01-15T18:59:30Z

LGTM

johnsaigle · 2019-01-15T19:18:48Z

@HenriRabalais I did a PHPCS for you

johnsaigle · 2019-01-16T21:41:27Z

modules/battery_manager/php/battery_manager.class.inc needs PHPCS

HenriRabalais · 2019-01-17T21:49:31Z

modules/battery_manager/php/testprovisioner.class.inc

+          LEFT JOIN test_names t USING (Test_name)
+          LEFT JOIN subproject s USING (SubprojectID)
+          LEFT JOIN psc p USING (CenterID)
+        ORDER BY t.Full_name",


I'm not sure why, but this doesn't seem to be working. Every time the query is made, the rows appear in a different order.

@HenriRabalais Have you tried sorting by a column that is part of the result rows? ex: testName

I'll give it a try

HenriRabalais · 2019-01-17T21:49:58Z

modules/battery_manager/help/battery_manager.md

@@ -0,0 +1,54 @@
+# Battery Manager
+*For Rida: Add what each value in the test_battery does and what happens if it's left empty*


modules/battery_manager/php/battery_manager.class.inc

modules/battery_manager/php/testoptionscontroller.class.inc

jsx/FilterableDataTable.js

modules/battery_manager/js/batteryManagerIndex.js

modules/battery_manager/js/editEntry.js

modules/battery_manager/php/testprovisioner.class.inc

modules/battery_manager/php/testcontroller.class.inc

jsx/DataTable.js

Addressed

PapillonMcGill · 2019-01-28T18:17:49Z

restarted Travis as error doesn't seem to make sense with change

johnsaigle · 2019-01-28T19:06:33Z

Needs PHPCS

FILE: .../aces/Loris/modules/battery_manager/php/testcontroller.class.inc
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 91 | WARNING | Line exceeds 85 characters; contains 86 characters
----------------------------------------------------------------------

HenriRabalais · 2020-02-04T20:07:07Z

@ridz1208 should be fixed.

made sure everything is functional minor format change fixed error in code reverted Modal form to original state fixed user perm rel file

ridz1208 · 2020-02-06T20:34:29Z

Testing results:

When entering a new test, 4 fields are required but only 2 of them have the red star next to the field name (see images)
Required fields do not have validation on the JS side, it's only on submission that they are validated backend.
if you try editing an entry, don't change anything and hit submit. You get an error message, test duplicate... it should not give an error if you click edit and don't change anything
Getting a test duplicate error when test is not duplicated (see images below)
activate/deactivate is not working (when you click it it works but no change in the database)
Instrument_order field should not be taken into consideration when checking for duplicate (ie if all the rest of the fields are the same but the order is different, it's still a conflict)
the min/max age and instrument order filters behave as textual search not integer matching (not sure if that is fixable)
change status and edit metadata appear in the downloaded CSV data (it doesnt make sens in the CSV to be there)

ridz1208 · 2020-02-11T14:27:37Z

    ERROR in ./modules/battery_manager/jsx/batteryManagerIndex.js
    Module Error (from ./node_modules/eslint-loader/index.js):
    
    /var/www/loris/modules/battery_manager/jsx/batteryManagerIndex.js
      255:1  warning  Line 255 exceeds the maximum line length of 80  max-len
      453:3  error    Missing JSDoc for parameter 'test'              valid-jsdoc
    
    ✖ 2 problems (1 error, 1 warning)

… battery_manager_2018_10_16

driusan

There are some minor problems but I don't think they're blockers, and they can be addressed later.

driusan · 2020-02-12T18:53:48Z

modules/battery_manager/jsx/batteryManagerIndex.js

+window.addEventListener('load', () => {
+  ReactDOM.render(
+    <BatteryManagerIndex
+      testEndpoint={`${loris.BaseURL}/battery_manager/testendpoint/`}


I don't really understand why these URLs have "endpoint" in the name

driusan · 2020-02-12T18:54:28Z

modules/battery_manager/help/battery_manager.md

@@ -0,0 +1,117 @@
+# Battery Manager


This doesn't seem to follow the style guidelines in docs/HelpStyleGuide.md

HenriRabalais added Category: Feature PR or issue that aims to introduce a new feature [branch] major labels Dec 20, 2018

driusan reviewed Dec 21, 2018

View reviewed changes

SQL/New_patches/2018_07_23_battery_manager_permissions.sql Outdated Show resolved Hide resolved

driusan previously requested changes Dec 21, 2018

View reviewed changes

HenriRabalais force-pushed the battery_manager_2018_10_16 branch from 770c1b8 to 5589429 Compare December 21, 2018 19:18

johnsaigle added the State: Needs work PR awaiting additional work by the author to proceed label Jan 2, 2019

zaliqarosli reviewed Jan 2, 2019

View reviewed changes

jsx/Form.js Outdated Show resolved Hide resolved

johnsaigle added the State: Discussion required PR or issue that requires the resolution of a discussion with the relevant parties to proceed label Jan 4, 2019

PapillonMcGill reviewed Jan 7, 2019

View reviewed changes

PapillonMcGill reviewed Jan 8, 2019

View reviewed changes

modules/battery_manager/test/TestPlan.md Outdated Show resolved Hide resolved

modules/battery_manager/test/TestPlan.md Outdated Show resolved Hide resolved

HenriRabalais force-pushed the battery_manager_2018_10_16 branch from 47a8dae to 0a21444 Compare January 15, 2019 22:10

HenriRabalais force-pushed the battery_manager_2018_10_16 branch from a5811c8 to 8c30756 Compare January 17, 2019 20:42

HenriRabalais commented Jan 17, 2019

View reviewed changes

modules/battery_manager/php/testoptionscontroller.class.inc Outdated Show resolved Hide resolved

HenriRabalais removed State: Discussion required PR or issue that requires the resolution of a discussion with the relevant parties to proceed State: Needs work PR awaiting additional work by the author to proceed labels Jan 17, 2019

zaliqarosli previously requested changes Jan 21, 2019

View reviewed changes

jsx/FilterableDataTable.js Show resolved Hide resolved

modules/battery_manager/js/batteryManagerIndex.js Outdated Show resolved Hide resolved

modules/battery_manager/js/editEntry.js Outdated Show resolved Hide resolved

xlecours reviewed Jan 21, 2019

View reviewed changes

modules/battery_manager/php/testprovisioner.class.inc Outdated Show resolved Hide resolved

xlecours reviewed Jan 21, 2019

View reviewed changes

modules/battery_manager/php/testcontroller.class.inc Outdated Show resolved Hide resolved

cmadjar reviewed Jan 25, 2019

View reviewed changes

jsx/DataTable.js Outdated Show resolved Hide resolved

HenriRabalais added the "Help! I don't understand Travis!" PR is having a beef with TRAVIS. Someone needs to help label Jan 25, 2019

ridz1208 added the State: Needs work PR awaiting additional work by the author to proceed label Jan 30, 2020

HenriRabalais removed the State: Needs work PR awaiting additional work by the author to proceed label Feb 4, 2020

HenriRabalais force-pushed the battery_manager_2018_10_16 branch from f6849b8 to dcc02e8 Compare February 4, 2020 20:11

Create module Battery Manager

5165a39

made sure everything is functional minor format change fixed error in code reverted Modal form to original state fixed user perm rel file

HenriRabalais force-pushed the battery_manager_2018_10_16 branch from dcc02e8 to 5165a39 Compare February 4, 2020 20:14

HenriRabalais added 6 commits February 6, 2020 17:29

addressed Rida's Review

0a1f696

made more adjustments to address review

ee5f391

addressed final parts of review

2a0553e

changed edit functionality

8289005

cleaned up code

17c39e8

further cleaning

c2d7533

ridz1208 and others added 7 commits February 11, 2020 09:39

fix travis

2548e65

fix 2

582aade

Merge branch 'master' into battery_manager_2018_10_16

2d2c478

cleaned more code

0ff8186

Merge remote-tracking branch 'origin/battery_manager_2018_10_16' into…

45828cd

… battery_manager_2018_10_16

fixed help text on form

f099d57

finalized code clean up

71a6d34

ridz1208 approved these changes Feb 12, 2020

View reviewed changes

ridz1208 added Passed manual tests PR has been successfully tested by at least one peer Release: Add to release notes PR whose changes should be highlighted in the release notes labels Feb 12, 2020

added permissions

6a062ec

driusan approved these changes Feb 12, 2020

View reviewed changes

fixed SQL error

bf4f6ea

driusan merged commit f3ccb31 into aces:master Feb 13, 2020

ridz1208 added this to the 23.0.0 milestone Feb 25, 2020

HenriRabalais deleted the battery_manager_2018_10_16 branch May 7, 2020 16:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Battery Manager] - New Module #4221

[Battery Manager] - New Module #4221

HenriRabalais commented Dec 20, 2018 •

edited

Loading

driusan commented Dec 21, 2018

driusan left a comment

HenriRabalais commented Jan 7, 2019

PapillonMcGill commented Jan 15, 2019

johnsaigle commented Jan 15, 2019

johnsaigle commented Jan 16, 2019

HenriRabalais Jan 17, 2019

xlecours Jan 21, 2020

HenriRabalais Jan 21, 2020

HenriRabalais Jan 17, 2019

PapillonMcGill commented Jan 28, 2019

johnsaigle commented Jan 28, 2019

HenriRabalais commented Feb 4, 2020

ridz1208 commented Feb 6, 2020 •

edited

Loading

ridz1208 commented Feb 11, 2020

driusan left a comment

driusan Feb 12, 2020

driusan Feb 12, 2020

		@@ -0,0 +1,54 @@
		# Battery Manager
		For Rida: Add what each value in the test_battery does and what happens if it's left empty

[Battery Manager] - New Module #4221

[Battery Manager] - New Module #4221

Conversation

HenriRabalais commented Dec 20, 2018 • edited Loading

Summary of Functions

Features

driusan commented Dec 21, 2018

driusan left a comment

Choose a reason for hiding this comment

HenriRabalais commented Jan 7, 2019

PapillonMcGill commented Jan 15, 2019

johnsaigle commented Jan 15, 2019

johnsaigle commented Jan 16, 2019

HenriRabalais Jan 17, 2019

Choose a reason for hiding this comment

xlecours Jan 21, 2020

Choose a reason for hiding this comment

HenriRabalais Jan 21, 2020

Choose a reason for hiding this comment

HenriRabalais Jan 17, 2019

Choose a reason for hiding this comment

PapillonMcGill commented Jan 28, 2019

johnsaigle commented Jan 28, 2019

HenriRabalais commented Feb 4, 2020

ridz1208 commented Feb 6, 2020 • edited Loading

ridz1208 commented Feb 11, 2020

driusan left a comment

Choose a reason for hiding this comment

driusan Feb 12, 2020

Choose a reason for hiding this comment

driusan Feb 12, 2020

Choose a reason for hiding this comment

HenriRabalais commented Dec 20, 2018 •

edited

Loading

ridz1208 commented Feb 6, 2020 •

edited

Loading